-
Notifications
You must be signed in to change notification settings - Fork 1
Add CI workflow explorer and validation #111
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add CI workflow explorer and validation #111
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
💡 Codex Review
Here are some automated review suggestions for this pull request.
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| const loadWorkflowFile = useCallback(async (relativePath: string) => { | ||
| if (!repository?.localPath || !window.electronAPI?.readWorkflowFile) { | ||
| return; | ||
| } | ||
| setIsWorkflowLoading(true); | ||
| setWorkflowError(null); | ||
| try { | ||
| const result = await window.electronAPI.readWorkflowFile({ repoPath: repository.localPath, relativePath }); | ||
| if (result?.success && typeof result.content === 'string') { | ||
| setWorkflowEditorContent(result.content); | ||
| setWorkflowOriginalContent(result.content); | ||
| } else { | ||
| setWorkflowError(result?.error || 'Unable to load workflow.'); | ||
| } | ||
| } catch (error: any) { | ||
| logger.error('Failed to read workflow file', { repoId: repository?.id ?? null, relativePath, error: error?.message || error }); | ||
| setWorkflowError(error?.message || 'Unable to read workflow.'); | ||
| } finally { | ||
| setIsWorkflowLoading(false); | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Guard workflow editor against stale loads
When two workflow files are selected back‑to‑back, both loadWorkflowFile calls run concurrently and each one blindly writes its result into the editor once the IPC read resolves (lines 3434‑3453). There is no check that the file whose contents just arrived is still the one shown in the UI. If the slower request finishes last, the editor will display the contents of the previously selected workflow while the header still shows the new file, so any edits/saves are applied to the wrong file. Please gate the state updates on the current selection (e.g. compare relativePath with selectedWorkflowPathRef.current or cancel the previous load) so that stale reads cannot clobber the active document.
Useful? React with 👍 / 👎.
| ipcMain.handle('commit-workflow-files', async (_event, { repo, filePaths, message }: { repo: Repository; filePaths: string[]; message: string }): Promise<{ success: boolean; error?: string }> => { | ||
| if (!repo || repo.vcs !== VcsTypeEnum.Git) { | ||
| return { success: false, error: 'Workflow commits are only supported for Git repositories.' }; | ||
| } | ||
| try { | ||
| const settings = await readSettings(); | ||
| const gitCmd = getExecutableCommand(VcsTypeEnum.Git, settings); | ||
| const sanitized = filePaths.map(fp => resolveWorkflowPath(repo.localPath, fp).relative); | ||
| const quotedFiles = sanitized.map(fp => `"${fp.replace(/"/g, '\\"')}"`).join(' '); | ||
| await execAsync(`${gitCmd} add ${quotedFiles}`, { cwd: repo.localPath }); | ||
| await execAsync(`${gitCmd} commit -m "${message.replace(/"/g, '\\"')}"`, { cwd: repo.localPath }); | ||
| await execAsync(`${gitCmd} push`, { cwd: repo.localPath }); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Commit message allows shell injection
The workflow commit handler builds shell commands with interpolated user input: execAsync(${gitCmd} commit -m "${message.replace(/"/g, '\"')}", …) (electron/main.ts lines 1696‑1707). Because execAsync uses child_process.exec, the command runs through /bin/sh, so characters like ` or $() in the commit message will be evaluated by the shell before git sees them (git commit -m "$(touch /tmp/pwn)" will execute touch). This lets anyone who edits a workflow run arbitrary shell commands on the host just by typing a crafted commit message. Please switch to spawn/execFile with argument arrays or fully escape shell metacharacters so that the message is passed to git verbatim.
Useful? React with 👍 / 👎.
Summary
.github/workflowsfiles, edit YAML, fork recommended templates, and push workflow commits without leaving the modalyamllintandact, and document the new capability in the changelogTesting
Codex Task